Skip to content

fix(guidance): hide geocode-bar when starting navigation so Stop is tappable (#175)#176

Merged
jasoneplumb merged 2 commits intomainlinefrom
fix/hide-geocode-bar-on-navigate
Apr 26, 2026
Merged

fix(guidance): hide geocode-bar when starting navigation so Stop is tappable (#175)#176
jasoneplumb merged 2 commits intomainlinefrom
fix/hide-geocode-bar-on-navigate

Conversation

@jasoneplumb
Copy link
Copy Markdown
Owner

Summary

  • The geocode-bar (position: fixed; bottom: 0; height: 60vh; z-index: 1500) in peek state covers the bottom ~130 px of the viewport above all Leaflet controls.
  • The bar's drag handle has pointer-events: auto and spans the bar's full width, sitting at the top of the visible peek area — at the same y-coordinate as the guidance pill's Stop button at the bottom of the bottom-left control cluster.
  • Result: every tap intended for Stop hits the handle instead. User reported the button "doesn't respond like it's not a button".

Fix

Call hideGeocodeBar() from the geocode-bar's Navigate click handler before startGuidance() runs. Once guidance starts, the bar's information is redundant (the pill shows the destination) and keeping it open just blocks taps on the pill's controls.

Why this is the right scope

The other entry points to guidance don't have the overlap:

  • Search dropdown's "Navigate here" (sheet-result__nav-btn) starts guidance directly without showing the bar.
  • The compass / hillshade / locate buttons are unrelated.

So narrowly hiding the bar from its own Navigate click handler addresses every flow that reaches the bug.

Closes #175.

Test plan

  • npm run type-check clean
  • npm run lint clean
  • npm test clean (633 tests)
  • Manual: long-press → tap Navigate (in bar) → guiding state → Stop button responds
  • Manual: search → "Go to location" → bar opens → tap Navigate → Stop responds
  • Regression: tapping Copy / drag-handle still works in peek state when no guidance is active

🤖 Generated with Claude Code

The geocode-bar in peek state covers the bottom ~130 px of the
viewport (z-index 1500) above all Leaflet controls. Its drag handle
has pointer-events: auto and spans the bar's full width, sitting at
the top of the visible peek area.

The bottom-left cluster (locate, guidance-panel, zoom, scale,
version, attribution) stacks 140-160 px tall, so the guidance pill's
Stop button is at the same y-coordinate as the bar's handle. The
handle catches every tap intended for Stop — the user reported the
button "doesn't respond like it's not a button".

Once Navigate is tapped from the bar, the destination is in the
guidance pill and the bar is redundant. Hiding it removes the
overlap entirely.

Closes #175

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jasoneplumb jasoneplumb added bug Something isn't working P1 Critical — fix immediately review-requested Triggers Claude code review labels Apr 26, 2026
@claude
Copy link
Copy Markdown

claude Bot commented Apr 26, 2026

Code Review

Overview

This is a minimal, well-scoped fix for a real mobile UX bug: the geocode-bar's drag handle (with pointer-events: auto) sits at the same y-coordinate as the guidance pill's Stop button after navigation starts, silently stealing taps. Adding hideGeocodeBar() before startGuidance() is the correct and natural fix — the bar has nothing more to offer once guidance is active.


Code Quality

Correctness
The fix is placed exactly right — hiding the bar before startGuidance() means the UI is clean when the guidance pill appears. hideGeocodeBar()snapTo('hidden') is idempotent, so calling it when the bar is already hidden (edge case) is safe.

Scope
The PR description's reasoning about why the other entry points (sheet-result__nav-btn, long-press without the bar) don't exhibit the same overlap is sound. The narrow fix is appropriate.

Minor: comment length ⚠️
CLAUDE.md says "multi-line comment blocks — one short line max". The 4-line block is informative, but it could be collapsed to a single line and still convey the essential non-obvious constraint:

// Bar's drag-handle overlaps the guidance pill's Stop button at this y-coord (#175).
hideGeocodeBar();

The issue number is enough for future readers to trace the full backstory. Not a blocker, but worth tightening.


Test Coverage

  • Automated suite (633 tests) passes — appropriate, since this interaction is a CSS/DOM positioning issue that's not realistically unit-testable.
  • Manual test plan covers the happy path, the alternate flow ("Go to location" → bar opens → Navigate), and the regression case (drag-handle still works when guidance is inactive). That's the right set.

No concerns on

  • Performance (no hot path touched)
  • Security (no user-facing data flows changed)
  • TypeScript / lint (already green per test plan)

Verdict

Approve with nit. The logic is correct, the scope is tight, and the PR description thoroughly explains the root cause. Only suggestion is collapsing the comment to a single line per project style. Ready to merge once the manual checklist items are ticked off.

Per PR #176 review nit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jasoneplumb jasoneplumb merged commit 6bc52ff into mainline Apr 26, 2026
2 checks passed
@jasoneplumb jasoneplumb deleted the fix/hide-geocode-bar-on-navigate branch April 26, 2026 06:07
@claude
Copy link
Copy Markdown

claude Bot commented Apr 26, 2026

Code Review

Summary: Minimal, well-targeted bug fix. Adds hideGeocodeBar() before startGuidance() in the geocode-bar's Navigate click handler to clear the z-index overlap that was swallowing taps on the guidance pill's Stop button.


Correctness ✅

  • The fix is in exactly the right place — the barNavBtn click handler is the only path that starts guidance while the geocode-bar can still be visible. The PR description's scope analysis is accurate: the dropdown's .sheet-result__nav-btn path skips the bar entirely, so it correctly receives no change.
  • hideGeocodeBar() is synchronous (snapTo('hidden')), so currentPinLatLng and currentPinLabel are still fully in scope when startGuidance consumes them — no ordering hazard.
  • Consistent with the existing pattern: hideGeocodeBar() is already called in every other dismiss path (lines 658, 688, 710, 740).

One minor observation

If startGuidance fails (e.g. Valhalla returns no route), the geocode-bar will have already been dismissed and the user loses the reverse-geocoded pin context. This is the same behaviour as the other dismiss paths, so it's consistent — but worth a conscious choice. The alternative would be to call hideGeocodeBar() only on success (inside the guidance-started callback). Given that the toast message already signals failure and the user can long-press again to reinstate the pin, the current approach seems fine; just flagging it in case you hadn't considered it.

Code style ✅

The comment explains the non-obvious why (z-index/layout collision, issue reference) rather than the what — exactly right per project conventions.

Tests ✅

No unit-testable logic changed; the fix is a DOM event handler interaction. The test plan's manual checklist covers the affected flows, and the existing 633-test suite passes.


Verdict: Looks good to merge. The fix is correct, minimal, and consistent with the rest of the file.

jasoneplumb added a commit that referenced this pull request Apr 26, 2026
…n on Navigate here (#177) (#178)

* fix(geocoding): make Navigate tappable in peek + close search dropdown on Navigate here

Two related taps were silently swallowed.

1. Geocode-bar Navigate button missing from peek pointer-events
   opt-in. The bar in peek state has pointer-events: none and only
   handle/copy/close/addr were explicitly opted back in. On mobile
   Safari the implicit child-receives-events behaviour for a
   pointer-events: none parent is unreliable, so .geocode-bar__nav
   silently dropped taps. Add it to the opt-in list.

   Without Navigate firing, hideGeocodeBar() from #176 never ran,
   so the bar stayed visible and the guidance pill's Stop button
   was still under the bar's drag handle.

2. Search dropdown's Navigate here handler started guidance without
   closing the dropdown. The dropdown sits at z-index 2000 above
   the guidance pill and on smaller viewports can overlap its
   action area; also a UX nit (stale chrome). Add hideDropdown()
   to the click handler.

Closes #177

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: drop issue ref from inline comment per CLAUDE.md

Per PR #178 review nit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Test <test@test.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working P1 Critical — fix immediately review-requested Triggers Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: guidance Stop button doesn't respond after using bar's Navigate (geocode-bar handle steals tap)

1 participant